fix(miner): fail closed when a chat-action handler throws#7041
Closed
shin-core wants to merge 1 commit into
Closed
fix(miner): fail closed when a chat-action handler throws#7041shin-core wants to merge 1 commit into
shin-core wants to merge 1 commit into
Conversation
dispatchChatAction wrapped its paramsValidator call in a try/catch
("fail closed") but left the next line, await registered.handler(request),
unguarded -- a throwing handler became an unhandled rejection instead of
the module's typed { ok, status } contract every other failure path
returns.
Wrap the handler call in try/catch, returning
{ ok: false, status: "handler_error", action, error } on throw, mirroring
the paramsValidator catch above. The distinct handler_error status lets a
caller tell an execution failure from a validation failure.
Closes JSONbored#6989
superagent-security
Bot
added
the
contributor:flagged
Contributor
|
🚨 Contributor flagged. Click here for more info: Superagent Dashboard |
![superagent-security[bot]](https://avatars.githubusercontent.com/in/3287076?s=60&v=4){kind=small}
| // A handler that throws fails closed with the module's typed result shape (#6989), consistent with the | ||
| // paramsValidator catch above -- a distinct "handler_error" status lets a caller tell an execution | ||
| // failure apart from a params-validation failure. | ||
| return { ok: false, status: "handler_error", action, error: error instanceof Error ? error.message : String(error) }; |
Contributor
There was a problem hiding this comment.
P2: Handler error messages returned to caller may leak sensitive internal details
Raw handler error messages are returned to the caller, risking internal detail leakage.
Log the error internally and return a generic error code instead of the raw message.
AI prompt
Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.
<file name="packages/loopover-miner/lib/chat-action-dispatch.js">
<violation number="1" location="packages/loopover-miner/lib/chat-action-dispatch.js:81">
<priority>P2</priority>
<title>Handler error messages returned to caller may leak sensitive internal details</title>
<evidence>The catch block for registered.handler(request) returns `error: error instanceof Error ? error.message : String(error)` directly in the typed result object. If this function is invoked from a REST endpoint, internal error details such as database connection strings, file paths, or secrets could leak to external clients.</evidence>
<recommendation>Log the full error internally and return a generic error identifier (e.g., an error code or trace ID) to the caller instead of the raw error message.</recommendation>
</violation>
</file>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7041 +/- ##
===========================================
+ Coverage 62.24% 93.74% +31.50%
===========================================
Files 692 692
Lines 68748 68750 +2
Branches 18769 18769
===========================================
+ Hits 42793 64453 +21660
+ Misses 23075 3302 -19773
+ Partials 2880 995 -1885
Flags with carried forward coverage won't be shown. Click here to find out more.
|
loopover-orb
Bot
added
the
gittensor:bug
|
Important 🟪🟪🟪🟪🟪🟪🟪🟪🟪🟪🟪🟪 🔍 LoopOver is reviewing…AI analysis is in progress. This comment will update when the review is complete. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed · 🟪 Reviewing |
JSONbored
reviewed
Jul 17, 2026
JSONbored
left a comment
JSONbored
left a comment
Owner
There was a problem hiding this comment.
Outstanding superagent issue, closing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
dispatchChatAction(packages/loopover-miner/lib/chat-action-dispatch.js) wraps itsparamsValidatorcall in a try/catch — explicitly "fail closed" — but the next line,await registered.handler(request), was unguarded. A handler that throws (e.g. a network error from a governor action) would surface as an unhandled rejection instead of the module's own typed{ ok, status }contract that every other failure path already returns.This is latent today (the dashboard-chat REST endpoint that invokes it is still an open epic, #6839), but wiring it live would turn any handler throw into an unhandled rejection.
How
Wrap
registered.handler(request)in try/catch, returning{ ok: false, status: "handler_error", action, error }on throw — mirroring theparamsValidatorcatch immediately above. The distinct"handler_error"status (vs"invalid_params") lets a caller tell an execution failure apart from a validation failure. This matches the fail-closed catch pattern used by the package's other must-never-crash paths (pretooluse-hook.js:32,sentry.js:37,47).Validation
Tests added to the existing
test/unit/miner-chat-action-dispatch.test.ts: a handler throwing anErroryieldshandler_errorwith the message, and a non-Errorthrow is stringified — covering both sides of theinstanceof Errorternary. Confirmed both fail against the unfixed code (unhandled rejection) and pass with the fix; 100% branch coverage on the file.Closes #6989